Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Option to return more info from Stockfish in .get_top_moves() #115

Closed
wants to merge 3 commits into from
Closed

Option to return more info from Stockfish in .get_top_moves() #115

wants to merge 3 commits into from

Conversation

knutole
Copy link

@knutole knutole commented Sep 29, 2022 •
edited
Loading

In certain use-cases it is nice to get more information from .get_top_moves(), like WDL, selective depth, nodes, etc.

One use-case for this is when evaluating full games. If you want both top moves for n lines, and the WDL, you'd have to run the evaluation multiple times. Also, there is currently no way of getting info like nodes, time, nps, etc.

# unchanged 
moves = stockfish.get_top_moves(2)
print(moves)
# [
#   {   
#     'Centipawn': None,
#     'Mate': 1,
#     'Move': 'e1e8'
#   },
#   {
#     'Centipawn': None,
#     'Mate': 2,
#     'Move': 'c8e8'
#   }
# ]

# unchanged
moves = stockfish.get_top_moves(2, include_info=False)
print(moves)
# [
#   {   
#     'Centipawn': None,
#     'Mate': 1,
#     'Move': 'e1e8'
#   },
#   {
#     'Centipawn': None,
#     'Mate': 2,
#     'Move': 'c8e8'
#   }
# ]

# returns more info
moves = stockfish.get_top_moves(2, include_info=True)
print(moves)
# [
#   {   
#     'Centipawn': None,
#     'Mate': 1,
#     'Move': 'e1e8',
#     'MultiPVLine': '1',
#     'N/s': '10255014',
#     'Nodes': '5096742',
#     'SelectiveDepth': '2',
#     'Time': '497',
#     'WDL': '1000 0 0'
#   },
#   {
#     'Centipawn': None,
#     'Mate': 2,
#     'Move': 'c8e8',
#     'MultiPVLine': '2',
#     'N/s': '10255014',
#     'Nodes': '5096742',
#     'SelectiveDepth': '4',
#     'Time': '497',
#     'WDL': '1000 0 0'
#   }
# ]

@knutole knutole changed the title Add include_info=True flag to .get_top_moves() Add option to return more info from MultiPV lines in .get_top_moves() Sep 29, 2022
@github-actions
Copy link

github-actions bot commented Sep 29, 2022 •
edited
Loading

Coverage report

The coverage rate went from 86% to 87% ⬆️
The branch rate is 79%.

85% of new lines are covered.

Diff Coverage details (click to unfold)

stockfish/models.py

86% of new lines are covered (87% of the complete file).

Missing lines: 601

@knutole
Add include_info=True flag to .get_top_moves()
8a5ebdc 
lint with black

add dict type

more elaborate typing

type

type this

ignore type

fixed

Separete moves, info in Tuple if include_info=True

lint with black

add test

revert; add test

fix test

cleanup
@knutole knutole changed the title Add option to return more info from MultiPV lines in .get_top_moves() Option to return more info from Stockfish in .get_top_moves() Sep 29, 2022
@kieferro
Copy link
Contributor

kieferro commented Mar 7, 2023

Thank you for your contribution to the project.

This project is no longer maintained in this repo but on this fork. (More information about this can be found here).

We are in the process of transferring the PRs to the fork one by one. Your two PRs (this and #116) would be next. That's why we would be very happy if you could close your two PRs on this repo and open them again for the fork. We will then review and merge them. Thanks.

@kieferro
Copy link
Contributor

kieferro commented Apr 4, 2023

Hi @knutole. I just wanted to check back and see how things are. If you currently don't have the time/possibility to reopen the PR in the other repo, that's no problem at all. But please just let us know so we can plan. We wanted to merge your PRs first so we don't get any conflicts and we are currently still waiting with the other PRs. But as I said, if it's not a good fit for you right now, just let us know. I don't want to rush you or make you extra work.
Thank you very much

@knutole
Copy link
Author

knutole commented Apr 4, 2023

Hi @knutole. I just wanted to check back and see how things are. If you currently don't have the time/possibility to reopen the PR in the other repo, that's no problem at all. But please just let us know so we can plan. We wanted to merge your PRs first so we don't get any conflicts and we are currently still waiting with the other PRs. But as I said, if it's not a good fit for you right now, just let us know. I don't want to rush you or make you extra work. Thank you very much

Hi @kieferro. Nice to see the project being maintained. I have moved the two PR's to the new repo. 👍

knutole and others added 2 commits April 5, 2023 00:17
@knutole @kieferro
Update stockfish/models.py
c201434 
Add suggested cleanup

Co-authored-by: kieferro <81773954+kieferro@users.noreply.github.com>
@knutole
Update readme with include_info flag
0906d2e
@kieferro kieferro mentioned this pull request Apr 20, 2023
Merged
@knutole
Copy link
Author

knutole commented Apr 20, 2023

Closed by py-stockfish#16

@knutole knutole closed this Apr 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@knutole @kieferro